-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CMake: Do not print installation messages for up-to-date files #60832
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors r+ p=1 since this may cause PRs to fail |
📌 Commit ec2f3e13e035d13985abf2c6f6aee8059b1bf0c2 has been approved by |
I would personally prefer if this were only applied to the location necessary for llvm, all the others should be inconsequential and/or handled already. Could the comment be expanded as well as to why it is applied, not just what it is? |
The reason is that The Travis issue with overflowing logs is a trigger for enabling it rather than the reason. |
The option is useless if the install directory is certainly fresh and empty. For cmake that's run from |
It makes sense yeah that it's a better default, and I'm not disagreeing at all. All our build scripts are already complicated and this is just one more thing to understand whenever reading them, and it's basically easier to read zero code than it is to reason later "did someone add this for correctness?" or having to preserve this through all future changes. I would rather than this be surgical and only apply, with a comment, to where it matters. Changing |
OK, the messages in the failed job were produced by LLVM and LLD builds, so I reduced the changes to those only. @bors r=Mark-Simulacrum |
📌 Commit 3646b3c has been approved by |
CMake: Do not print installation messages for up-to-date files Closes rust-lang#60830
CMake: Do not print installation messages for up-to-date files Closes rust-lang#60830
CMake: Do not print installation messages for up-to-date files Closes #60830
☀️ Test successful - checks-travis, status-appveyor |
Closes #60830